Remove registry preload and avoid updating sources prematurely instead

So registry preloading was added[1] to avoid updating the root path
multiple times unnecessarily, since that's an expensive operation. But
with Package::from_path, we can get the root package just from a
manifest and a config without having to update the whole path source.

So now we don't have to remember to preload the registry if we've
already updated a source and want to use the registry later. Instead, we
just avoid loading the source initially and let the registry do it when
it needs to -- which is now in resolve_with_previous.

This does cause one tested error message to happen slightly later than
it used to, but it still happens.

[1] - ef8c651af

diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs
index 273bb5ed56b92c1c951253728861163e6c96e08b..de00c40b5a6b824d2031ac7770bdba84e78cfb00 100644 (file)
--- a/src/cargo/core/registry.rs
+++ b/src/cargo/core/registry.rs
@@ -147,11 +147,6 @@ impl<'cfg> PackageRegistry<'cfg> {
         Ok(())
     }
 
-    pub fn preload(&mut self, id: &SourceId, source: Box<Source + 'cfg>) {
-        self.sources.insert(id, source);
-        self.source_ids.insert(id.clone(), (id.clone(), Kind::Locked));
-    }
-
     pub fn add_sources(&mut self, ids: &[SourceId]) -> CargoResult<()> {
         for id in ids.iter() {
             try!(self.load(id, Kind::Locked));

diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs
index f177e5e7e4e3496b9c83f4745b3cda6a0ab1a217..e9191304417694e48ae98d13284f847f9e4905ba 100644 (file)
--- a/src/cargo/ops/cargo_compile.rs
+++ b/src/cargo/ops/cargo_compile.rs
@@ -32,7 +32,6 @@ use core::{Source, SourceId, PackageSet, Package, Target, PackageId};
 use core::{Profile, TargetKind};
 use core::resolver::Method;
 use ops::{self, BuildOutput, ExecEngine};
-use sources::{PathSource};
 use util::config::{ConfigValue, Config};
 use util::{CargoResult, internal, human, ChainError, profile};
 
@@ -87,20 +86,16 @@ pub fn compile<'a>(manifest_path: &Path,
                    -> CargoResult<ops::Compilation<'a>> {
     debug!("compile; manifest-path={}", manifest_path.display());
 
-    let mut source = try!(PathSource::for_path(manifest_path.parent().unwrap(),
-                                               options.config));
-    // TODO: Move this into PathSource
-    let package = try!(source.root_package());
+    let package = try!(Package::for_path(manifest_path, options.config));
     debug!("loaded package; package={}", package);
 
     for key in package.manifest().warnings().iter() {
         try!(options.config.shell().warn(key))
     }
-    compile_pkg(&package, Some(Box::new(source)), options)
+    compile_pkg(&package, options)
 }
 
 pub fn compile_pkg<'a>(package: &Package,
-                       source: Option<Box<Source + 'a>>,
                        options: &CompileOptions<'a>)
                        -> CargoResult<ops::Compilation<'a>> {
     let CompileOptions { config, jobs, target, spec, features,
@@ -125,12 +120,6 @@ pub fn compile_pkg<'a>(package: &Package,
 
     let (packages, resolve_with_overrides, sources) = {
         let mut registry = PackageRegistry::new(config);
-        if let Some(source) = source {
-            registry.preload(package.package_id().source_id(), source);
-        } else {
-            try!(registry.add_sources(&[package.package_id().source_id()
-                                               .clone()]));
-        }
 
         // First, resolve the package's *listed* dependencies, as well as
         // downloading and updating all remotes and such.

diff --git a/src/cargo/ops/cargo_fetch.rs b/src/cargo/ops/cargo_fetch.rs
index 34e6bb27d74dda7f77dd2b42574a67da9c683633..612d88c05ada4f37f3b2b389e7a2d6b497da52f9 100644 (file)
--- a/src/cargo/ops/cargo_fetch.rs
+++ b/src/cargo/ops/cargo_fetch.rs
@@ -1,19 +1,14 @@
 use std::path::Path;
 
 use core::registry::PackageRegistry;
-use core::{Source, PackageId};
+use core::{Package, PackageId};
 use ops;
-use sources::PathSource;
 use util::{CargoResult, Config, human, ChainError};
 
 /// Executes `cargo fetch`.
 pub fn fetch(manifest_path: &Path, config: &Config) -> CargoResult<()> {
-    let mut source = try!(PathSource::for_path(manifest_path.parent().unwrap(),
-                                               config));
-    let package = try!(source.root_package());
-
+    let package = try!(Package::for_path(manifest_path, config));
     let mut registry = PackageRegistry::new(config);
-    registry.preload(package.package_id().source_id(), Box::new(source));
     let resolve = try!(ops::resolve_pkg(&mut registry, &package));
 
     let ids: Vec<PackageId> = resolve.iter().cloned().collect();

diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs
index c3de1dc435b8dd66d3ff6073d06c9c2b1c54c830..5edca1acef75f1f78262d63b450c55553023c339 100644 (file)
--- a/src/cargo/ops/cargo_generate_lockfile.rs
+++ b/src/cargo/ops/cargo_generate_lockfile.rs
@@ -3,10 +3,9 @@ use std::path::Path;
 
 use core::PackageId;
 use core::registry::PackageRegistry;
-use core::{Resolve, SourceId};
+use core::{Resolve, SourceId, Package};
 use core::resolver::Method;
 use ops;
-use sources::{PathSource};
 use util::config::{Config};
 use util::{CargoResult, human};
 
@@ -19,11 +18,8 @@ pub struct UpdateOptions<'a> {
 
 pub fn generate_lockfile(manifest_path: &Path, config: &Config)
                          -> CargoResult<()> {
-    let mut source = try!(PathSource::for_path(manifest_path.parent().unwrap(),
-                                               config));
-    let package = try!(source.root_package());
+    let package = try!(Package::for_path(manifest_path, config));
     let mut registry = PackageRegistry::new(config);
-    registry.preload(package.package_id().source_id(), Box::new(source));
     let resolve = try!(ops::resolve_with_previous(&mut registry, &package,
                                                   Method::Everything,
                                                   None, None));
@@ -33,9 +29,7 @@ pub fn generate_lockfile(manifest_path: &Path, config: &Config)
 
 pub fn update_lockfile(manifest_path: &Path,
                        opts: &UpdateOptions) -> CargoResult<()> {
-    let mut source = try!(PathSource::for_path(manifest_path.parent().unwrap(),
-                                               opts.config));
-    let package = try!(source.root_package());
+    let package = try!(Package::for_path(manifest_path, opts.config));
 
     let previous_resolve = match try!(ops::load_pkg_lockfile(&package)) {
         Some(resolve) => resolve,
@@ -83,7 +77,6 @@ pub fn update_lockfile(manifest_path: &Path,
         None => to_avoid.extend(previous_resolve.iter()),
     }
 
-    registry.preload(package.package_id().source_id(), Box::new(source));
     let resolve = try!(ops::resolve_with_previous(&mut registry,
                                                   &package,
                                                   Method::Everything,

diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs
index 684c539fe04f98192a2a31f49420217d3daf7c0b..0988f00df527f518ef51a506423a3ea0aff33e93 100644 (file)
--- a/src/cargo/ops/cargo_package.rs
+++ b/src/cargo/ops/cargo_package.rs
@@ -178,7 +178,7 @@ fn run_verify(config: &Config, pkg: &Package, tar: &Path)
     let new_pkg = Package::new(new_manifest, &manifest_path);
 
     // Now that we've rewritten all our path dependencies, compile it!
-    try!(ops::compile_pkg(&new_pkg, None, &ops::CompileOptions {
+    try!(ops::compile_pkg(&new_pkg, &ops::CompileOptions {
         config: config,
         jobs: None,
         target: None,

diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
index 7f17321bd7d77bb1b8143401c6674aad604e668c..4657181bb385c2f268d74c5ee119710490120ca5 100644 (file)
--- a/src/cargo/ops/resolve.rs
+++ b/src/cargo/ops/resolve.rs
@@ -37,6 +37,9 @@ pub fn resolve_with_previous<'a>(registry: &mut PackageRegistry,
                                  to_avoid: Option<&HashSet<&'a PackageId>>)
                                  -> CargoResult<Resolve> {
 
+    try!(registry.add_sources(&[package.package_id().source_id()
+                                       .clone()]));
+
     // Here we place an artificial limitation that all non-registry sources
     // cannot be locked at more than one revision. This means that if a git
     // repository provides more than one package, they must all be updated in

diff --git a/tests/test_cargo_compile_path_deps.rs b/tests/test_cargo_compile_path_deps.rs
index 87174cc4220f355c470336283c2d1db943bd2ffd..b8b89fe6515cdbfcd9ba22f9658c45fa9bf53688 100644 (file)
--- a/tests/test_cargo_compile_path_deps.rs
+++ b/tests/test_cargo_compile_path_deps.rs
@@ -513,8 +513,12 @@ test!(error_message_for_missing_manifest {
     assert_that(p.cargo_process("build"),
                 execs()
                 .with_status(101)
-                .with_stderr(&format!("Could not find `Cargo.toml` in `{}`\n",
-                                      p.root().join("src").join("bar").display())));
+                .with_stderr(&format!("\
+Unable to update file://[..]
+
+Caused by:
+  Could not find `Cargo.toml` in `{}`
+", p.root().join("src").join("bar").display())));
 
 });